Skip to content

Add Enum Ratio to PlutusTx stdlib. #7406

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Nov 25, 2025
Merged

Add Enum Ratio to PlutusTx stdlib. #7406

merged 1 commit into from
Nov 25, 2025
+219 −53

Conversation

@bezirg
Copy link
Contributor

@bezirg bezirg commented Nov 6, 2025 •
edited
Loading

Added an Enum Rational instance that mimicks that of GHC and Haskell report.

I also removed the toGHC/fromGHC from plutustx since they don't work on-chain.
I think it is straightforward to re-create these two-functions for off-chain use, simply by a combination of numerator and denominator.

Pre-submit checklist:

  • Branch
    • Tests are provided (if possible)
    • Commit sequence broadly makes sense
    • Key commits have useful messages
    • Changelog fragments have been written (if appropriate)
    • Relevant tickets are mentioned in commit messages
    • Formatting, PNG optimization, etc. are updated
  • PR
    • (For external contributions) Corresponding issue exists and is linked in the description
    • Targeting master unless this is a cherry-pick backport
    • Self-reviewed the diff
    • Useful pull request description
    • Reviewer requested

@bezirg bezirg requested a review from kwxm November 6, 2025 08:00
@bezirg bezirg self-assigned this Nov 6, 2025
@bezirg bezirg force-pushed the bezirg/enum-ratio branch 4 times, most recently from 591ff50 to 5b497a7 Compare November 6, 2025 10:24
@bezirg bezirg marked this pull request as ready for review November 6, 2025 11:54
SeungheonOh
SeungheonOh reviewed Nov 7, 2025
View reviewed changes
Copy link
Collaborator

@SeungheonOh SeungheonOh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good, but I think it's okay to keep fromGHC and toGHC as is. We have other helpers in plutus-tx and plutus-ledger-api that won't work on-chain but is quite useful.

@bezirg
Copy link
Contributor Author

bezirg commented Nov 7, 2025

Changes look good, but I think it's okay to keep fromGHC and toGHC as is. We have other helpers in plutus-tx and plutus-ledger-api that won't work on-chain but is quite useful.

I thought we are trying to keep plutus-tx on-chain only. But I am open to discussions about this and if so , bring it back.
If it is the case to keep it then I propose to change the name fromGHC to something more generic like fromHS or fromHaskell.

@SeungheonOh
Copy link
Collaborator

SeungheonOh commented Nov 7, 2025

I thought we are trying to keep plutus-tx on-chain only.

I was mostly about keeping plutus-core out from packages that defines plinth types. I don't have a firm opinion on things that interfaces with regular Haskell as they don't often add any new dependencies anyways

@bezirg bezirg requested a review from SeungheonOh November 24, 2025 10:08
@bezirg
Copy link
Contributor Author

bezirg commented Nov 24, 2025

I put the fromGHC / toGHC back and renamed them to from/toHaskellRatio

@bezirg bezirg changed the title Add Enum Ratio to PlutusTx stdlib. Remove Ratio: half/toGHC/fromGHC Add Enum Ratio to PlutusTx stdlib. Nov 24, 2025
SeungheonOh
SeungheonOh approved these changes Nov 24, 2025
View reviewed changes
Copy link
Collaborator

@SeungheonOh SeungheonOh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Left some comments

@bezirg bezirg enabled auto-merge (squash) November 25, 2025 09:35
@bezirg bezirg merged commit 5354949 into master Nov 25, 2025
5 checks passed
@bezirg bezirg deleted the bezirg/enum-ratio branch November 25, 2025 11:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@SeungheonOh SeungheonOh SeungheonOh approved these changes

@kwxm kwxm Awaiting requested review from kwxm

Assignees

@bezirg bezirg

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@bezirg @SeungheonOh